Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Check all env vars and suggest alternatives for misspelled keys #1719

Merged
merged 18 commits into from
Feb 10, 2024
Merged

Check all env vars and suggest alternatives for misspelled keys #1719

merged 18 commits into from
Feb 10, 2024

Conversation

DL6ER
Copy link
Member

@DL6ER DL6ER commented Oct 31, 2023 •
edited
Loading

What does this implement/fix?

This PR adds checking for all FTLCONF_* environment variables. We log used ones, invalid ones (e.g. 123 for something expecting a boolean), and unknown keys (ignored). In the latter case, we try to suggest a meaningful alternative to aid typo fixing.

Example environment:

FTLCONF_misc_nice=-12
FTLCONF_misc_privacyllll=-12
FTLCONF_dns_prt=123
FTLCONF_debug_api=not_a_bool
FTLCONF_dns_upstrrrr=1,2,3
FTLCONF_debug_ApI=true

leading to the following log output:
(removed, see updated output in comment below)

When no environment variables are defined, nothing environmental variables-related is logged at all.

Related issue or feature (if applicable): N/A

Pull request in docs with documentation (if applicable): N/A

By submitting this pull request, I confirm the following:

  1. I have read and understood the contributors guide, as well as this entire template. I understand which branch to base my commits and Pull Requests against.
  2. I have commented my proposed changes within the code.
  3. I am willing to help maintain this change if there are issues with it later.
  4. It is compatible with the EUPL 1.2 license
  5. I have squashed any insignificant commits. (git rebase)

Checklist:

  • The code change is tested and works locally.
  • I based my code and PRs against the repositories developmental branch.
  • I signed off all commits. Pi-hole enforces the DCO for all contributions
  • I signed all my commits. Pi-hole requires signatures to verify authorship
  • I have read the above and my PR is ready for review.

@DL6ER DL6ER requested a review from a team October 31, 2023 18:36
@DL6ER DL6ER changed the title Tweak/env vars list Improve env var handling Oct 31, 2023
@DL6ER DL6ER mentioned this pull request Oct 31, 2023
Closed
5 tasks
@DL6ER DL6ER removed the request for review from a team October 31, 2023 20:01
@DL6ER DL6ER marked this pull request as draft October 31, 2023 20:01
yubiuser
yubiuser reviewed Oct 31, 2023
View reviewed changes
Copy link
Member

@yubiuser yubiuser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

FTLCONF_debug_api is invalid, using default

Would be nice if the log would tell us, why it is invalid. E.g.

FTLCONF_debug_api is invalid (expecting bool, string set), using default

@DL6ER
Copy link
Member Author

DL6ER commented Nov 1, 2023

The latest commits change the output to

2023-11-01 06:52:11.637 [1067602M] INFO: 6 FTLCONF environment variables found (1 used, 1 invalid, 4 ignored)
2023-11-01 06:52:11.637 [1067602M] WARNING: [?] FTLCONF_debug_ApI is unknown, did you mean any of these?
2023-11-01 06:52:11.637 [1067602M] WARNING:     - FTLCONF_debug_api
2023-11-01 06:52:11.637 [1067602M] WARNING: [?] FTLCONF_dns_upstrrrr is unknown, did you mean any of these?
2023-11-01 06:52:11.637 [1067602M] WARNING:     - FTLCONF_dns_upstreams
2023-11-01 06:52:11.637 [1067602M] ERROR:   [✗] FTLCONF_debug_api is invalid (not of type bool), using default
2023-11-01 06:52:11.638 [1067602M] WARNING: [?] FTLCONF_dns_HOSTS is unknown, did you mean any of these?
2023-11-01 06:52:11.638 [1067602M] WARNING:     - FTLCONF_dns_hosts
2023-11-01 06:52:11.638 [1067602M] WARNING: [?] FTLCONF_misc_privacy is unknown, did you mean any of these?
2023-11-01 06:52:11.638 [1067602M] WARNING:     - FTLCONF_misc_privacylevel
2023-11-01 06:52:11.638 [1067602M] WARNING:     - FTLCONF_misc_nice
2023-11-01 06:52:11.638 [1067602M] INFO:    [✓] FTLCONF_misc_nice is used

Furthermore, we extend suggesting mistyped keys to the CLI (pihole-FTL --config) as well:

$ pihole-FTL --config dbg.all
Unknown config option dbg.all, did you mean:
 - debug.all

$ pihole-FTL --config deg.t
Unknown config option deg.t, did you mean:
 - debug.tls

$ pihole-FTL --config dns.upsrrrrr
Unknown config option dns.upsrrrrr, did you mean:
 - dns.upstreams

$ pihole-FTL --config misc.privaaaacy
Unknown config option misc.privaaaacy, did you mean:
 - misc.privacylevel

@DL6ER DL6ER requested review from yubiuser and a team November 1, 2023 15:38
@DL6ER DL6ER marked this pull request as ready for review November 1, 2023 15:38
@github-actions GitHub Actions
Copy link

github-actions bot commented Nov 1, 2023

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@github-actions GitHub Actions
Copy link

github-actions bot commented Nov 1, 2023

Conflicts have been resolved.

@DL6ER DL6ER force-pushed the tweak/env_vars_list branch 2 times, most recently from 4ef87a2 to 3e095da Compare November 1, 2023 17:23
@DL6ER
Copy link
Member Author

DL6ER commented Nov 1, 2023

Rebased on latest development-v6

@DL6ER DL6ER changed the title Improve env var handling Check all env vars and suggest alternatives for misspelled keys Nov 1, 2023
@github-actions GitHub Actions
Copy link

github-actions bot commented Nov 8, 2023

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@github-actions GitHub Actions
Copy link

github-actions bot commented Nov 8, 2023

Conflicts have been resolved.

@github-actions GitHub Actions
Copy link

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@github-actions GitHub Actions
Copy link

Conflicts have been resolved.

@github-actions GitHub Actions
Copy link

This pull request has conflicts, please resolve those before we can evaluate the pull request.

DL6ER added 7 commits November 23, 2023 10:10
@DL6ER
Add the (fuzzy) Bitap algorithm and suggest up to three alternatives …
0fbf0d3 
…for typos

Signed-off-by: DL6ER <[email protected]>
@DL6ER
Use WARNING for unknown and ERROR for invalid env vars, also rename "…
5c20f40 
…ERR" to "ERROR" when printing errors (we also have INFO, WARNING, DEBUG, NOTICE, ALERT, ...)

Signed-off-by: DL6ER <[email protected]>
@DL6ER
Explicitly log why env vars were invalid in the overview
a23dad8 
Signed-off-by: DL6ER <[email protected]>
@DL6ER
Also suggest possible alternatives for mistyped config keys on the CLI
01d1644 
Signed-off-by: DL6ER <[email protected]>
@DL6ER
Log config file statistics after writing
e8039ed 
Signed-off-by: DL6ER <[email protected]>
@DL6ER
Standardize CLI --config exit codes and add tests
fc4bceb 
Signed-off-by: DL6ER <[email protected]>
@DL6ER
Also suggest keys starting with the given key. This can be helpful to…
a92e656 
…, e.g. find misc.privacylevel is misc.priv is entered as the Hamming distance to misc.nice is shorter

Signed-off-by: DL6ER <[email protected]>
@DL6ER DL6ER force-pushed the tweak/env_vars_list branch from ef41397 to a92e656 Compare November 23, 2023 09:12
@github-actions GitHub Actions
Copy link

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@github-actions GitHub Actions
Copy link

Conflicts have been resolved.

@github-actions GitHub Actions
Copy link

github-actions bot commented Dec 9, 2023

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@github-actions GitHub Actions
Copy link

github-actions bot commented Dec 9, 2023

Conflicts have been resolved.

@github-actions GitHub Actions
Copy link

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@github-actions GitHub Actions
Copy link

Conflicts have been resolved.

@DL6ER DL6ER requested a review from PromoFaux January 13, 2024 09:47
PromoFaux
PromoFaux approved these changes Feb 10, 2024
View reviewed changes
Copy link
Member

@PromoFaux PromoFaux left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

pihole  | 2024-02-10 08:25:00.174 [146M] WARNING: [?] FTLCONF_webserver_aip_passwordddd is unknown, did you mean any of these?
pihole  | 2024-02-10 08:25:00.174 [146M] WARNING:     - FTLCONF_webserver_api_password

@DL6ER DL6ER merged commit 5940c62 into development-v6 Feb 10, 2024
17 checks passed
@DL6ER DL6ER deleted the tweak/env_vars_list branch February 10, 2024 15:21
@PromoFaux PromoFaux mentioned this pull request Feb 18, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@PromoFaux PromoFaux PromoFaux approved these changes

@yubiuser yubiuser Awaiting requested review from yubiuser

Assignees
No one assigned
Labels
Pi-hole v6.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@DL6ER @PromoFaux @yubiuser